Skip to content

Conversation

@bewithgaurav
Copy link
Collaborator

Work Item / Issue Reference

AB#<WORK_ITEM_ID>

GitHub Issue: #318


Summary

This pull request introduces improvements to the handling of string encoding in the getinfo method for SQL Server connections, adds support for profiling builds in the Windows build script, and enhances test coverage for string decoding. The most important changes are grouped below:

String Decoding Improvements

  • The getinfo method in connection.py now attempts to decode string results from SQL Server using multiple encodings in order: UTF-16LE (Windows default), UTF-8, and Latin-1. This improves robustness when handling driver responses and avoids silent data corruption by returning None if all decoding attempts fail.

Test Coverage

  • Added a new test test_getinfo_string_encoding_utf16 in test_003_connection.py to verify that string values returned by getinfo are properly decoded from UTF-16, contain no null bytes, and are non-empty, helping catch encoding mismatches early.

Build Script Cleanup

  • Removed redundant logic from build.bat related to copying the msvcp140.dll redistributable, simplifying the post-build process.

@github-actions github-actions bot added the pr-size: medium Moderate update size label Nov 21, 2025
@github-actions
Copy link

github-actions bot commented Nov 21, 2025

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

74%


📈 Total Lines Covered: 5260 out of 7022
📁 Project: mssql-python


Diff Coverage

Diff: main...HEAD, staged and unstaged changes

  • mssql_python/connection.py (100%)

Summary

  • Total: 5 lines
  • Missing: 0 lines
  • Coverage: 100%

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.logger_bridge.hpp: 58.8%
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.cpp: 66.2%
mssql_python.row.py: 66.2%
mssql_python.helpers.py: 67.5%
mssql_python.pybind.connection.connection.cpp: 73.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.connection.py: 83.9%
mssql_python.cursor.py: 84.3%
mssql_python.__init__.py: 84.9%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: medium Moderate update size labels Nov 21, 2025
@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: small Minimal code update labels Dec 11, 2025
@bewithgaurav bewithgaurav marked this pull request as ready for review December 16, 2025 05:49
Copilot AI review requested due to automatic review settings December 16, 2025 05:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes a UTF-16 encoding bug in the getinfo() method that was causing null bytes to appear in string values returned from SQL Server. The fix implements a proper encoding fallback mechanism (UTF-16LE → UTF-8) to handle ODBC's wide-character API responses, adds comprehensive test coverage for the encoding scenarios, and removes redundant DLL copying logic from the Windows build script.

Key Changes:

  • Replaced single UTF-8 decoding attempt with a multi-encoding fallback strategy (UTF-16LE first, then UTF-8)
  • Added four new test cases covering UTF-16 decoding success, UTF-8 fallback, encoding failure, and null byte detection
  • Removed obsolete msvcp140.dll redistribution logic from the build script

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
mssql_python/connection.py Implements UTF-16LE decoding with UTF-8 fallback in getinfo() method for proper ODBC string handling
tests/test_003_connection.py Adds comprehensive test coverage for UTF-16 encoding scenarios including primary path, fallback path, and failure cases
mssql_python/pybind/build.bat Removes redundant Visual C++ redistributable DLL copying logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jahnvi480
jahnvi480 previously approved these changes Dec 16, 2025
@bewithgaurav bewithgaurav merged commit f8723e9 into main Dec 17, 2025
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: medium Moderate update size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants